Make flower-server start server with HTTPS by default
#2591
Conversation
There was a problem hiding this comment.
PR looks good!
run_driver_api and run_fleet_api are the targets for two additional commands that allow users to start the Driver API and the Fleet API independent of each other (see pyproject.toml:
Those two commands will have to support --insecure and --certificates, too. The easiest way to do it is probably through _add_args_common.
…nto make-https-default
e2e/certificates/ca.crt
Outdated
| @@ -0,0 +1,30 @@ | |||
| -----BEGIN CERTIFICATE----- | |||
There was a problem hiding this comment.
I would not commit these here but rather generate them each time the test runs and delete them afterwards. It's often easier to commit them, but at the same time, I would prefer to avoid having any secret keys or certificates even for testing committed to prevent any possibility someone might accidentally use them somewhere.
There was a problem hiding this comment.
@tanertopal wdyt, should we also move the certificates directory into the bare-https dir? They are only required for that single test
There was a problem hiding this comment.
If they are only required here, then let's move it. The directory could also contain the script to generate the certificates.
Co-authored-by: Taner Topal <[email protected]>
Co-authored-by: Taner Topal <[email protected]>
--insecureargument to start server without HTTPS.--certificatesargument to accept paths to (CA certificate, server certificate, and server private key)certificatesargument to some functions to facilitate the above changes.bare-httpsto test starting server with SSL-enabled.e2e/certificates/for testing purposes.e2e/test.shande2e/test_driver.sh.Note
flower-servercommand will fail without either--insecureor--certificates a b c.--insecureis enabled,--certificateswill be ignored. (This is mentioned in the help message.)e2e/bare-https/simulation.pyis empty because simulation does not require any certificate.grpc-rereandgrpc-bidiand I am not sure how to unify the three.